Sdk upload tutorial #33
Conversation
| @@ -71,8 +71,28 @@ following into the CORS field | |||
| (*it's basically saying that we are allowing GET, | |||
| POST and PUT requests from any Allowed Origin with any Allowed Header*) | |||
|
|
|||
| + Finally we need to add a policy to the bucket to make it public readable. Click | |||
There was a problem hiding this comment.
@jackcarlisle might be worth clarifying that this makes the files uploaded to the bucket viewable in browsers.
| In order to upload to our S3 bucket, we have to use the AWS SDK. Install it using | ||
| the following command: | ||
|
|
||
| `$ npm install aws-sdk --save` |
There was a problem hiding this comment.
we tend to exclude the $ from shell/terminal commands to avoid confusion.
| ### Step 4 - Implement the SDK | ||
|
|
||
| First you'll need to set some environment variables. These are as follows: | ||
|
|
There was a problem hiding this comment.
Tip: include links to "learn-xyz" tutorials when mentioning concepts that might not be familiar to everyone (new developers) in this case:
If you are new to Environment Variables see: github.com/dwyl/learn-environment-variables
Also, you could recommend that people store these two in an .env file in their project.
| We're going to create a credentials file at `~/.aws/credentials`. To do this we | ||
| take the following steps: | ||
|
|
||
| * `$ cd` this takes us back to the root of our file system |
There was a problem hiding this comment.
I'd recommend people open a _new terminal window_ to perform these actions so they can close it at the end and go back to their project directory in the original terminal. ...
Tell people to pwd first so they can get back to their project's working directory after they set up their ~/.aws/credentials file ...
|
|
||
| First you'll need to set some environment variables. These are as follows: | ||
|
|
||
| `export S3_REGION=<YOUR_REGION>` |
There was a problem hiding this comment.
prefer all AWS-based Environment Variables to be prefixed with AWS_ e.g: AWS_S3_REGION ... yes, it's quite safe to assume that there's only one env var called S3_REGION but no harm in being explicit.
| function upload (file, filename, callback) { | ||
| // creating our new filename | ||
| var filenameHex = | ||
| filename.split('.')[0] + |
There was a problem hiding this comment.
some people use . (dots) in their filenames e.g: my.amazing.holiday.photo.jpg
which would end up my.HEXSTRING.jpg so there's a relatively low risk of filename collision. but given that you are getting the extname below, you could:
var ext = '.' + path.extname(filename);
var filenameHex = filename.replace(ext, '') +
crypto.randomBytes(8).toString('hex') + ext;
| var params = {Bucket: bucket, Key: filenameHex, Body: file} | ||
| // SDK upload to S3 | ||
| s3Bucket.upload(params, function (err, data) { | ||
| if (err) console.log(err, data) |
There was a problem hiding this comment.
handling errors in hapi apps: https://github.com/dwyl/hapi-error#handleerror-everywhere
this could be:
var handleError = require('hapi-error').handleError;
s3Bucket.upload(params, function (err, data) {
handleError(error, data); // let hapi-error display the appropriate error message
return callback(err, result);
}
| } | ||
| ``` | ||
|
|
||
| #### We've now set up our S3 upload function! |
There was a problem hiding this comment.
does this need to be a an h4 (sub-headding) ?
There was a problem hiding this comment.
I feel as though it helps to separate the sections. I can remove it if you'd like
| var filename = request.payload.filename | ||
| // upload the file to S3 | ||
| s3.upload(file, filename, function (err, data) { | ||
| if (err) console.log(err) |
There was a problem hiding this comment.
here you can do request.handleError(err, err);
(instead of having an if statement and having to write a another test for it...)
| </html> | ||
| ``` | ||
|
|
||
| Create the javascript file that we're including in our html file and call it |
There was a problem hiding this comment.
@jackcarlisle this works but requires JS... we can do better (progressive enhancement) ... 😉
It's totally my fault for not being specific when we discussed the requirements.
Please create an issue in the backlog to add a progressive enhancement version.
There was a problem hiding this comment.
@nelsonic issue added!
| @@ -0,0 +1,21 @@ | |||
| <?xml version="1.0" encoding="UTF-8" ?> | |||
There was a problem hiding this comment.
the test reports should probably be excluded from the repo ... thoughts?
(do they add value to the reader?)
|
@jackcarlisle the code looks superb. 👀 👍
We can merge this PR but please acknowledge these notes first. thanks!! 🎉 |
|
@nelsonic I'll add the screenshots to the .gitignore and when I get a chance I'll take a look at a progressive enhancement demo! |
|
@jackcarlisle thanks for updating that. If you don't mind can you please add the section to the readme of "contributing" to help others avoid adding images/binary files in the future...? dwyl/contributing#22 (thanks) |
ready for review
Server side S3 SDK example #32